Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add smartptrs and atomics #5

Merged
merged 5 commits into from
Jul 22, 2021
Merged

Add smartptrs and atomics #5

merged 5 commits into from
Jul 22, 2021

Conversation

planetis-m
Copy link
Contributor

@planetis-m planetis-m commented Jul 21, 2021 •
edited
Loading

So this PR add smartptrs after passing a review round here nim-lang/Nim#18450
They now depend on new atomics #4 Other changes include the addition of newSharePtr/newUniquePtr without data parameter and the accompanying []= overloads. As discussed nim-lang/Nim#18450 (comment)

Bug planetis-m/sync#13 affects pointers that contain ref objects, only when the user tries to reference them in a local variable let a: ref Foo = mysharedptr[] not sure how to disallow this.

@planetis-m
Copy link
Contributor Author

Unrelated?

 /home/runner/work/threading/threading/nim/lib/nimbase.h:542:1: note: in expansion of macro ‘NIM_STATIC_ASSERT’
 NIM_STATIC_ASSERT(sizeof(NI) == sizeof(void*) && NIM_INTBITS == sizeof(NI)*8, "");
 ^~~~~~~~~~~~~~~~~
In file included from /home/runner/.cache/nim/tatomics_d/stdlib_assertions.nim.c:11:0:
/home/runner/work/threading/threading/nim/lib/nimbase.h:271:35: error: static assertion failed: ""
 #define NIM_STATIC_ASSERT(x, msg) _Static_assert((x), msg)
                                   ^
/home/runner/work/threading/threading/nim/lib/nimbase.h:542:1: note: in expansion of macro ‘NIM_STATIC_ASSERT’
 NIM_STATIC_ASSERT(sizeof(NI) == sizeof(void*) && NIM_INTBITS == sizeof(NI)*8, "");
 ^~~~~~~~~~~~~~~~~

@narimiran
Copy link
Member

Unrelated?

Yes, it was there before. We need to look into that separately.

@Araq
Copy link
Member

Araq commented Jul 22, 2021

I'm merging this, other CI failures can be fixed later.

@Araq Araq merged commit d0c3573 into nim-lang:master Jul 22, 2021
@planetis-m planetis-m mentioned this pull request Jul 23, 2021
Open
@PhilippMDoerner PhilippMDoerner mentioned this pull request Jan 5, 2024
Closed
@planetis-m planetis-m mentioned this pull request Aug 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Araq Araq Araq left review comments

@arnetheduck arnetheduck arnetheduck left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@planetis-m @narimiran @Araq @arnetheduck